[Tune] Fix checkpoint directory assignment for new checkpoints created after restoring a function trainable #31231
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Justin Yu <[email protected]>
Signed-off-by: Justin Yu <[email protected]>
Signed-off-by: Justin Yu <[email protected]>
Signed-off-by: Justin Yu <[email protected]>
krfricke
approved these changes
Dec 21, 2022
Comment on lines
+130
to
+138
| result_queue: queue.Queue, | ||
| continue_semaphore: threading.Semaphore, | ||
| end_event: threading.Event, | ||
| training_iteration_func: Callable[[], int], | ||
| experiment_name: Optional[str] = None, | ||
| trial_name: Optional[str] = None, | ||
| trial_id: Optional[str] = None, | ||
| logdir: Optional[str] = None, | ||
| trial_resources: Optional[Union[Resources, PlacementGroupFactory]] = None, |
Signed-off-by: Justin Yu <[email protected]>
…ored_checkpoint_idx
7 tasks
krfricke
pushed a commit
that referenced
this pull request
Jan 4, 2023
…31423) This PR is a follow-up to #31231 to save checkpoints to the correctly indexed directory upon restore. The "latest checkpoint ID" that's used to generate the next checkpoint directory (`checkpoint_0000<latest_checkpoint_id>`) is off by one when restoring an AIR trainer. Signed-off-by: Justin Yu <[email protected]>
AmeerHajAli
pushed a commit
that referenced
this pull request
Jan 12, 2023
…d after restoring a function trainable (#31231) This PR fixes checkpoint directory creation for restored function trainables to use the restored iteration instead of starting over from `checkpoint_000000`. Signed-off-by: Justin Yu <[email protected]>
AmeerHajAli
pushed a commit
that referenced
this pull request
Jan 12, 2023
…31423) This PR is a follow-up to #31231 to save checkpoints to the correctly indexed directory upon restore. The "latest checkpoint ID" that's used to generate the next checkpoint directory (`checkpoint_0000<latest_checkpoint_id>`) is off by one when restoring an AIR trainer. Signed-off-by: Justin Yu <[email protected]>
tamohannes
pushed a commit
to ju2ez/ray
that referenced
this pull request
Jan 25, 2023
…d after restoring a function trainable (ray-project#31231) This PR fixes checkpoint directory creation for restored function trainables to use the restored iteration instead of starting over from `checkpoint_000000`. Signed-off-by: Justin Yu <[email protected]> Signed-off-by: tmynn <[email protected]>
tamohannes
pushed a commit
to ju2ez/ray
that referenced
this pull request
Jan 25, 2023
…ay-project#31423) This PR is a follow-up to ray-project#31231 to save checkpoints to the correctly indexed directory upon restore. The "latest checkpoint ID" that's used to generate the next checkpoint directory (`checkpoint_0000<latest_checkpoint_id>`) is off by one when restoring an AIR trainer. Signed-off-by: Justin Yu <[email protected]> Signed-off-by: tmynn <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes checkpoint directory creation for restored function trainables to use the restored iteration instead of starting over from
checkpoint_000000.Why are these changes needed?
The
_StatusReporterthat handles checkpoint directory creation for function trainables keeps track of an_iterthat closely follows the Trainabletraining_iteration, and it's used to create checkpoint directories (ex:checkpoint_000000).Upon restoring a trial, this iteration is not restored properly and starts from 0, which results in a new checkpoint possibly overwriting an old one at the
checkpoint_000000path.The ticket below has more details, but the basic failure case:
checkpoint_000000checkpoint_000000Solution
Don't keep track of an
_iterseparately in the session - use the trainable's currenttraining_iterationinstead.Open Questions
training_iteration, so the first checkpoint is saved ascheckpoint_000001. Function trainables are 0-indexed. This PR could change it to make this consistent, and I think that the class trainable indexing makes more sense rather than always being 1-off with respect to the iteration number. Decision: This would break backwards compatibility when restoring experiments that have 0 indexed checkpoints, so this will be left for a future PR.Related issue number
Closes #29947
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.